fix(start): align request handler types with registered server context#7428
fix(start): align request handler types with registered server context#7428SeanCassiere wants to merge 11 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR aligns request handler types across the start framework packages by updating where the ChangesRequest Handler Type Alignment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
documentation
|
View your CI Pipeline Execution ↗ for commit d8feafb
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview3 package(s) bumped directly, 0 bumped as dependents. 🟩 Patch bumps
|
Bundle Size Benchmarks
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
SeanCassiere
changed the title
Merging this PR will not alter performance
Comparing 
Footnotes
|
SeanCassiere
force-pushed
the
refactor/adjust-types-for-start-register-declaration
branch
from
89310e0 to
d6b0635
Compare
SeanCassiere
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/start-server-core/src/createStartHandler.ts`:
- Around line 325-327: startRequestResolver is declared as
RequestHandler<Register> with requestOpts?: RequestOptions<Register>, but the
code later casts it to RequestHandler<TRegister>, which hides
RequestOptions<TRegister> constraints such as required server.requestContext;
instead make startRequestResolver generic over TRegister (or accept
RequestOptions<TRegister>) within createStartHandler so its signature matches
RequestHandler<TRegister> without casting—update the function/type parameters
(startRequestResolver, RequestOptions, RequestHandler, createStartHandler,
TRegister, Register) so the resolver uses the generic TRegister request option
types (including requestContext when present) and remove the unsafe cast.
In `@packages/start-server-core/src/request-response.ts`:
- Around line 125-128: The wrapper currently declares requestOpts?: any which
bypasses the RequestHandler<TRegister> contract and then re-establishes types
via a cast; change the wrapper signature to accept the strongly-typed options
instead of any so the wrapper itself matches RequestHandler<TRegister> (i.e.,
replace requestOpts?: any with the correct generic opts type for TRegister and
adjust the wrapper return type to Promise<Response> | Response as needed),
remove the subsequent as RequestHandler<TRegister> cast, and ensure the internal
call sites use the typed opts variable (references: the wrapper function taking
(request: Request, requestOpts), the RequestHandler<TRegister> type and the
TRegister generic).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7286b653-e8ba-4a37-ae1f-5478603a1e1f
📒 Files selected for processing (8)
.changeset/thick-sloths-write.mdpackages/react-start/src/default-entry/server.tspackages/solid-start/src/default-entry/server.tspackages/start-server-core/src/createStartHandler.tspackages/start-server-core/src/index.tsxpackages/start-server-core/src/request-handler.tspackages/start-server-core/src/request-response.tspackages/vue-start/src/default-entry/server.ts
✅ Files skipped from review due to trivial changes (2)
- packages/start-server-core/src/index.tsx
- .changeset/thick-sloths-write.md
| return (( | ||
| request: Request, | ||
| requestOpts?: any, | ||
| ): Promise<Response> | Response => { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify unsafe typing in wrapper signatures
rg -n -C2 'requestOpts\?: any|as RequestHandler<|RequestHandlerParameters<' packages/start-server-core/src/request-response.tsRepository: TanStack/router
Length of output: 342
requestOpts?: any bypasses the handler type guarantees (and is masked by a cast).
In packages/start-server-core/src/request-response.ts, the wrapper takes requestOpts?: any (line 127) and then relies on as RequestHandler<TRegister> (line 146) to restore typing. This defeats strict type safety for the TRegister-specific opts contract. Replace any with the correct typed parameters for TRegister and remove/avoid the cast by aligning the wrapper’s parameter/return types with RequestHandler<TRegister] directly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/start-server-core/src/request-response.ts` around lines 125 - 128,
The wrapper currently declares requestOpts?: any which bypasses the
RequestHandler<TRegister> contract and then re-establishes types via a cast;
change the wrapper signature to accept the strongly-typed options instead of any
so the wrapper itself matches RequestHandler<TRegister> (i.e., replace
requestOpts?: any with the correct generic opts type for TRegister and adjust
the wrapper return type to Promise<Response> | Response as needed), remove the
subsequent as RequestHandler<TRegister> cast, and ensure the internal call sites
use the typed opts variable (references: the wrapper function taking (request:
Request, requestOpts), the RequestHandler<TRegister> type and the TRegister
generic).
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…t-start Add a Start-owned Register interface that extends router-core's Register, then export and consume it from `start-client-core`. This lets declarations against `@tanstack/react-start`, including `server.requestContext`, flow into Start-specific type consumers instead of requiring users to augment `@tanstack/router-core` or `@tanstack/react-router` directly. Router-core Register augmentations remain compatible because Start's Register extends the core Register.
Import Register from `@tanstack/start-client-core` in `createStartHandler` instead of `@tanstack/router-core`. This makes the server handler default to Start's augmentation surface, allowing `@tanstack/react-start` Register declarations such as `server.requestContext` to flow into RequestHandler typing.
…rt's Register type
Use the shared router-core Register for Start request handler types while preserving the documented custom server entry shape. Request options now derive required context from Register.server.requestContext, but framework ServerEntry wrappers still accept the universal fetch handler form with optional request options.
SeanCassiere
force-pushed
the
refactor/adjust-types-for-start-register-declaration
branch
from
73a9c6d to
bfdf4bb
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
… import Fixing the augmented `server.requestContext` typing only needs the default-entry `ServerEntry` to import `Register` from the framework start package (the module users augment), not the router package. Module augmentation flows only when the consuming type imports `Register` from the same specifier and that module is in the type graph. Revert the broader, unnecessary churn back to main: - start-server-core: request-handler, createStartHandler, request-response signatures (RequestHandler/RequestOptions/RequestHandlerParameters) - e2e react-start import-protection test - changeset: drop start-client-core/start-server-core (now unchanged) Verified against built packages: e2e react/solid/vue basic `tsc --noEmit` pass; wrong context value and missing required context are rejected; start-client-core/start-server-core test:types pass.
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.
🎓 Learn more about Self-Healing CI on nx.dev
1 participant
In reference to #7399
Summary by CodeRabbit
Bug Fixes
Documentation